Don't package private library and typelib files separately. #456
Conversation
|
@Fantu look ok to you? Thanks |
|
Big thanks, from a fast look ok the move from shared to private and can make possible remove another difference from Debian packaging (done to solve policy issue for shared library). |
|
I tested it and found that fails on:
I did a fast search and tried GIRepository.Repository.prepend_library_path (after saw https://stackoverflow.com/questions/42631816/modify-gobject-load-library-path-for-python) but didn't work. |
libcscreensaver/meson.build
Outdated
| include_directories: inc, | ||
| cpp_args: '-DG_LOG_DOMAIN="CScreensaver"', | ||
| dependencies: libcscreensaver_deps, | ||
| install_dir: libexecdir, |
There was a problem hiding this comment.
It would probably be better to build with --libexecdir=libexec in debian/rules, and change cinnamon-screensaver-pam-helper, cs-backup-locker, girepository-1.0/CScreensaver-1.0.typelib and libcscreensaver.so so they're installed into libexecdir / 'cinnamon-screensaver' (and then adjust paths in debian/*.install appropriately).
The install_rpath would also need to be set to get_option('prefix') / libexecdir / 'cinnamon-screensaver' for this.
That way, these would all be installed into a private directory like /usr/libexec/cinnamon-screensaver/ across all platforms.
At the moment, the packaging in debian/ overrides the libexec directory to be cinnamon-screensaver-specific (perhaps only as a result of historical debhelper defaults), but on e.g. Arch- or Red Hat-derived platforms these files would be installed directly into /usr/libexec, and I don't think that's what you really intend.
There was a problem hiding this comment.
It's somewhat conventional to define a variable like pkglibexecdir = libexecdir / 'cinnamon-screensaver', and then use things like install_dir: pkglibexecdir to reduce repetition.
src/binfile.in
Outdated
| @@ -5,4 +5,7 @@ if [ "$XDG_SESSION_TYPE" = "wayland" ]; then | |||
| exit 1 | |||
| fi | |||
|
|
|||
| export GI_TYPELIB_PATH="@libexecdir@" | |||
There was a problem hiding this comment.
If you're installing the typelib as @libexecdir@/girepository-1.0/CScreensaver-1.0.typelib then you will need to use a path like @libexecdir@/girepository-1.0 here.
(Similarly if you add a .../cinnamon-screensaver/... path component as I suggest above, you'll need to take that into account here and in the LD_LIBRARY_PATH.)
It would be OK to put the typelib either in the same private directory as the .so, or in a girepository-1.0 subdirectory, but you'll need to choose one or the other and then be consistent about it.
There are two search paths that are important for finding a typelib that describes a private library. GObject-Introspection needs to be able to find the typelib. That's done by using the It also needs to be able to find the actual shared library. That's done by using the If your typelib and your shared library are both private, as they are in this PR, then you need to set up both search paths. |
mtwebster
force-pushed
the
no-libcs-package
branch
from
14bb827 to
55e20f4
Compare
|
Thanks, updated. I think I moved the typelib location (and didn't test it) just before opening this - it worked for me :) @Fantu I'm not sure I can get rid of the dummy package though - I tried what you suggested (adding to cinnamon-screensaver's Breaks) but the install fails as it can't remove the old package. If we just remove it without that Breaks, it installs fine, but you're still left with libcscreensaver (with its files in /usr/lib/x86_64-linux-gnu...) |
|
@smcv big thanks for all the information and advices @mtwebster the breaks should work and replaces/conflicts are not needed as file moved between the packages have different path. I tested it in Debian and worked (https://salsa.debian.org/cinnamon-team/cinnamon-screensaver/-/tree/experimental?ref_type=heads). Used and worked also for other packages, previous time was for cinnamon-control-center-goa I did fast test on Debian unstable using debomatic as repository There is only a minor thing spotted with lintian, library files are now executable (in previous build wasn't), probably is packaging that mark executable all files under /usr/libexec about these 2 lintian errors instead I think can be overrided as now we are using private libraries: |
This is a debhelper limitation (arguably a debhelper bug). If you'd prefer to make the Debian packaging set the
Yes. You're intentionally using private libraries via a |
|
@smcv thanks for the information, I had just already done it after seeing from a quick search that the sudo package also has a similar situation, so I fixed the permissions and added the override as you also wrote. https://salsa.debian.org/cinnamon-team/cinnamon-screensaver/-/commits/experimental Probably also review of this PR from maintainers of other distro can be good, for example @leigh123linux |
|
I honestly preferred this all in /usr/lib/**/cinnamon-screensaver - I guess I got the impression from this discussion that libexec was preferred by Debian - I'm ok with it here, and I suppose this makes transitioning away from the libcscreensaver0 package easier also. |
The reason I suggested |
|
lib path arch based is required for multiarch support and probably also for shared library package (I don't remember good). |
|
I didn't see @smcv reply when writing, his reply gives useful information. @mtwebster given the latest @smcv info where would you prefer to have between libdir and libexecdir? |
For public shared libraries like GLib or GTK, we always want to install below the architecture-specific For private shared libraries (like
I think part of the confusion here is that this source tree contains both the upstream project (cinnamon-screensaver and its Meson build system), and a set of downstream packaging files for Mint (the |
| export GI_TYPELIB_PATH="@typelibdir@" | ||
| export LD_LIBRARY_PATH="@pkglibdir@" |
There was a problem hiding this comment.
Can we use
| export GI_TYPELIB_PATH="@typelibdir@" | |
| export LD_LIBRARY_PATH="@pkglibdir@" | |
| export GI_TYPELIB_PATH="@typelibdir@${GI_TYPELIB_PATH:+:}$GI_TYPELIB_PATH" | |
| export LD_LIBRARY_PATH="@pkglibdir@${LD_LIBRARY_PATH:+:}$LD_LIBRARY_PATH" |
here? On NixOS we relies on e.g. GI_TYPELIB_PATH to discover the system libraries, it will be nice not to override the existing one (ref: GNOME/libsecret@2089893 and/or GNOME/gtk@29e6cc5). Without this the screensaver complains:
Traceback (most recent call last):
File "/nix/store/acsigjq0ps6isqvy4jn37xmka7gw1psb-cinnamon-screensaver-6.2.0/share/cinnamon-screensaver/cinnamon-screensaver-main.py", line 4, in <module>
gi.require_version('Gtk', '3.0')
File "/nix/store/whykd0srhqzgfsxkkgbi1bx8yx9qdaz4-python3-3.12.4-env/lib/python3.12/site-packages/gi/__init__.py", line 122, in require_version
raise ValueError('Namespace %s not available' % namespace)
ValueError: Namespace Gtk not available
There was a problem hiding this comment.
Shouldn't these just append to the standard path list? Why is this behavior different in NixOS?
There was a problem hiding this comment.
This is the same case on Guix 😂 On our side GOBJECT_INTROSPECTION_LIBDIR is a path like /nix/store/4cbm71y0q5r8i9r6jkx09vr22k55slsq-gobject-introspection-1.80.1/lib which is actually immutable and actually only contains typelibs from gobject-introspection. We have toolings to automatically wrap the program with the proper environment variables but that can't work if the program itself resets the env.
For example:
$ ls /nix/store/4cbm71y0q5r8i9r6jkx09vr22k55slsq-gobject-introspection-1.80.1/lib/girepository-1.0
cairo-1.0.typelib DBusGLib-1.0.typelib freetype2-2.0.typelib GL-1.0.typelib Vulkan-1.0.typelib xfixes-4.0.typelib xlib-2.0.typelib
DBus-1.0.typelib fontconfig-2.0.typelib GIRepository-2.0.typelib libxml2-2.0.typelib win32-1.0.typelib xft-2.0.typelib xrandr-1.3.typelib
$ ls /nix/store/hrqbw86y0rid5qpxcy3s6sj97qgq23hb-xapp-2.8.5/lib/girepository-1.0
XApp-1.0.typelib
|
@mtwebster you want to do other changes or improvements to this PR? I want to use it on Debian, and I was wondering if I should wait any longer. |
mtwebster
force-pushed
the
no-libcs-package
branch
from
f926042 to
e982e22
Compare
|
Sorry, I'll merge it today - are you certain I don't need a transitional package for libcscreensaver0? |
|
On the PR I didn't added remove of the transitional package as you wrote didn't work:
I already tried in Debian and worked as it should and like other times I used it: obviously the version you install will have to be greater than the one installed for apt to do its job on Debian I added also the breaks for remove gir1.2-cscreensaver-1.0 (that was created to solve policy issue with shared library package) and was worked also its remove |
mtwebster
force-pushed
the
no-libcs-package
branch
from
e982e22 to
e3c3318
Compare
|
@mtwebster I did another look to documentation and should be correct the breaks (and only it as there is no file conflict in this case where the file path has changed)
It occurred to me that maybe you didn't do a full test but a quick build on the same device (instead of building in clean environments) and tried to install the cinnamon-screensaver package directly with dpkg (instead of use a test repository, even a local one can do it if necessary) and so it should rightly fail because libcscreensaver0 is still installed and configured. |
|
Yes correct - just local installs with dpkg, though I was basing my initial assumptions on the guidelines here: https://wiki.debian.org/PackageTransition (Case 6) I've bookmarked that new link, thanks. |
The cinnamon-screensaver package is already arch-specific, and libcscreensaver isn't used outside of this package. - Remove the libcscreensaver0 package. - Move helpers and libcscreensaver to a /usr/libexec private folder. - Clean up some variables, various build details.
There's really only one valid path, defined in the build.
mtwebster
force-pushed
the
no-libcs-package
branch
from
e3c3318 to
7884d90
Compare
|
The wiki page you linked is based on Debian Policy Manual page about declaring relationships between packages and have some useful examples. Debian Policy Manual is important and useful for packaging, I recommend you use it. |
The cinnamon-screensaver package is already arch-specific, and libcscreensaver isn't used outside of this package.
The libcscreensaver0 package will remain as an empty transitional package.
ref:
linuxmint/cinnamon#11999
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1059341#15